Bug 586374 – code does not follow documentation

Properly invalidate iterators in iter_next, iter_children,
iter_nth_child and iter_parent when FALSE is returned.  ListStore was
actually already mostly in order as the GSequence pointers are very
strictly checked, but to be a little stricter we've added stamp = 0
lines.  TreeStore did not invalidate its iterators, so stamp = 0 lines
were added where appropriate.  Unit tests have been added to check this.

Fixes bug 586374, reported by Mike Gemunde.

diff --git a/gtk/gtkliststore.c b/gtk/gtkliststore.c
index 470c9f82b188a7889caf1fa92c05823279d3f604..dd286298fe4b9803b7ae94d2b86561e0f6eaf8af 100644 (file)
--- a/gtk/gtkliststore.c
+++ b/gtk/gtkliststore.c
@@ -494,10 +494,16 @@ static gboolean
 gtk_list_store_iter_next (GtkTreeModel  *tree_model,
                          GtkTreeIter   *iter)
 {
+  gboolean retval;
+
   g_return_val_if_fail (GTK_LIST_STORE (tree_model)->stamp == iter->stamp, FALSE);
   iter->user_data = g_sequence_iter_next (iter->user_data);
 
-  return !g_sequence_iter_is_end (iter->user_data);
+  retval = g_sequence_iter_is_end (iter->user_data);
+  if (retval)
+    iter->stamp = 0;
+
+  return !retval;
 }
 
 static gboolean
@@ -509,7 +515,10 @@ gtk_list_store_iter_children (GtkTreeModel *tree_model,
   
   /* this is a list, nodes have no children */
   if (parent)
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 
   if (g_sequence_get_length (list_store->seq) > 0)
     {
@@ -518,7 +527,10 @@ gtk_list_store_iter_children (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -551,6 +563,8 @@ gtk_list_store_iter_nth_child (GtkTreeModel *tree_model,
   GtkListStore *list_store = (GtkListStore *) tree_model;
   GSequenceIter *child;
 
+  iter->stamp = 0;
+
   if (parent)
     return FALSE;
 
@@ -570,6 +584,7 @@ gtk_list_store_iter_parent (GtkTreeModel *tree_model,
                            GtkTreeIter  *iter,
                            GtkTreeIter  *child)
 {
+  iter->stamp = 0;
   return FALSE;
 }
 

diff --git a/gtk/gtktreestore.c b/gtk/gtktreestore.c
index 1fb178d375ae2b60d60a88cb1a7e29d7522744b8..97084084fb458261d2381bc7f24f80b5f1999163 100644 (file)
--- a/gtk/gtktreestore.c
+++ b/gtk/gtktreestore.c
@@ -620,7 +620,10 @@ gtk_tree_store_iter_next (GtkTreeModel  *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -646,7 +649,10 @@ gtk_tree_store_iter_children (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -708,7 +714,10 @@ gtk_tree_store_iter_nth_child (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -733,7 +742,10 @@ gtk_tree_store_iter_parent (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 

diff --git a/gtk/tests/liststore.c b/gtk/tests/liststore.c
index 588f725f3d5eab5c1ea85af6da9b28a9951a501a..6452fc290364bf34526b15690663468b807ef0f7 100644 (file)
--- a/gtk/tests/liststore.c
+++ b/gtk/tests/liststore.c
@@ -850,6 +850,72 @@ list_store_test_move_before_single (void)
   g_object_unref (store);
 }
 
+
+/* iter invalidation */
+
+static void
+list_store_test_iter_next_invalid (ListStore     *fixture,
+                                   gconstpointer  user_data)
+{
+  GtkTreePath *path;
+  GtkTreeIter iter;
+
+  path = gtk_tree_path_new_from_indices (4, -1);
+  gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), &iter, path);
+  gtk_tree_path_free (path);
+
+  g_assert (gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store),
+                                      &iter) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+static void
+list_store_test_iter_children_invalid (ListStore     *fixture,
+                                       gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_children (GTK_TREE_MODEL (fixture->store),
+                                          &child, &iter) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+list_store_test_iter_nth_child_invalid (ListStore     *fixture,
+                                        gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_nth_child (GTK_TREE_MODEL (fixture->store),
+                                           &child, &iter, 0) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+list_store_test_iter_parent_invalid (ListStore     *fixture,
+                                     gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &child);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &child) == TRUE);
+
+  g_assert (gtk_tree_model_iter_parent (GTK_TREE_MODEL (fixture->store),
+                                        &iter, &child) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+
 /* main */
 
 int
@@ -958,5 +1024,19 @@ main (int    argc,
   g_test_add_func ("/list-store/move-before-single",
                   list_store_test_move_before_single);
 
+  /* iter invalidation */
+  g_test_add ("/list-store/iter-next-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_next_invalid,
+              list_store_teardown);
+  g_test_add ("/list-store/iter-children-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_children_invalid,
+              list_store_teardown);
+  g_test_add ("/list-store/iter-nth-child-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_nth_child_invalid,
+              list_store_teardown);
+  g_test_add ("/list-store/iter-parent-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_parent_invalid,
+              list_store_teardown);
+
   return g_test_run ();
 }

diff --git a/gtk/tests/treestore.c b/gtk/tests/treestore.c
index e3a85c92fe583ed2a1bb1a30e377a168259f0767..c9dbcffba8cbefc2b2c5180d5099294f353039f1 100644 (file)
--- a/gtk/tests/treestore.c
+++ b/gtk/tests/treestore.c
@@ -853,6 +853,72 @@ tree_store_test_move_before_single (void)
   g_object_unref (store);
 }
 
+
+/* iter invalidation */
+
+static void
+tree_store_test_iter_next_invalid (TreeStore     *fixture,
+                                   gconstpointer  user_data)
+{
+  GtkTreePath *path;
+  GtkTreeIter iter;
+
+  path = gtk_tree_path_new_from_indices (4, -1);
+  gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), &iter, path);
+  gtk_tree_path_free (path);
+
+  g_assert (gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store),
+                                      &iter) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+static void
+tree_store_test_iter_children_invalid (TreeStore     *fixture,
+                                       gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_children (GTK_TREE_MODEL (fixture->store),
+                                          &child, &iter) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+tree_store_test_iter_nth_child_invalid (TreeStore     *fixture,
+                                        gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_nth_child (GTK_TREE_MODEL (fixture->store),
+                                           &child, &iter, 0) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+tree_store_test_iter_parent_invalid (TreeStore     *fixture,
+                                     gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &child);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &child) == TRUE);
+
+  g_assert (gtk_tree_model_iter_parent (GTK_TREE_MODEL (fixture->store),
+                                        &iter, &child) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+
 /* main */
 
 int
@@ -961,5 +1027,19 @@ main (int    argc,
   g_test_add_func ("/tree-store/move-before-single",
                   tree_store_test_move_before_single);
 
+  /* iter invalidation */
+  g_test_add ("/tree-store/iter-next-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_next_invalid,
+              tree_store_teardown);
+  g_test_add ("/tree-store/iter-children-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_children_invalid,
+              tree_store_teardown);
+  g_test_add ("/tree-store/iter-nth-child-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_nth_child_invalid,
+              tree_store_teardown);
+  g_test_add ("/tree-store/iter-parent-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_parent_invalid,
+              tree_store_teardown);
+
   return g_test_run ();
 }